🦋 Changeset detected

Latest commit: b3dbbad

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the integration-tests: skipped manually label to skip these checks.

Looks good! I noticed one small bug on the truncation story:

It seems that the third option doesn't load a tooltip, and the cursor changes rapidly 🤔

Looks good! I noticed one small bug on the truncation story:

Screen.Recording.2026-03-04.at.6.12.06.AM.mov
It seems that the third option doesn't load a tooltip, and the cursor changes rapidly 🤔

@TylerJDev Good catch, thanks!

This was exposed after we removed _privateDisableTooltip={!text} in 3488872#diff-21bd620cf266fd1b3eb8d73399ee032c406b9bf49eac6f598ad65c9389ddb515. Without it, ConditionalTooltip would mount/unmount <Tooltip> based on whether text was set, and that mount/unmount cycle was causing an infinite re-render loop for complex (non-string) descriptions.

The root cause was two separate useEffects in Description.tsx. the first extracted text content from the DOM and set computedTitle, which changed effectiveTitle, which triggered the second effect, which re-triggered the cycle. For simple string children this never happened because effectiveTitle was stable from the first render. I fixed it by merging the two effects into one effect that reads the DOM text and sets the truncatedText.

One question: do you think we should also add _privateDisableTooltip={!text}back so the Tooltip wrapper stays mounted at all times(when it's enabled)? That way the DOM tree shape is stable between setting the text in Description.

One question: do you think we should also add _privateDisableTooltip={!text}back so the Tooltip wrapper stays mounted at all times(when it's enabled)? That way the DOM tree shape is stable between setting the text in Description.

Thank you for the explanation! I guess with _privateDisableTooltip, are we only adding back the changes in the commit you referred to? Or are there other changes that would be included with this? 🤔

One question: do you think we should also add _privateDisableTooltip={!text}back so the Tooltip wrapper stays mounted at all times(when it's enabled)? That way the DOM tree shape is stable between setting the text in Description.

Thank you for the explanation! I guess with _privateDisableTooltip, are we only adding back the changes in the commit you referred to? Or are there other changes that would be included with this? 🤔

This would be the change:

-  if (!enabled || !text) {
+  if (!enabled) {
      return children
    }
    return (
-     <Tooltip ref={forwardedRef} text={text || ''} direction="e" delay="medium">
+     <Tooltip ref={forwardedRef} text={text || ''} direction="e" delay="medium" _privateDisableTooltip={!text}>
        {children}
      </Tooltip>
    )

Since text is set asynchronously in a useEffect or non-string descriptions, using if (!enabled || !text) to conditionally mount/unmount the Tooltip causes a DOM tree change between renders. With _privateDisableTooltip={!text}, the Tooltip stays mounted but disabled when text is empty, so the tree is always stable.

Since text is set asynchronously in a useEffect or non-string descriptions, using if (!enabled || !text) to conditionally mount/unmount the Tooltip causes a DOM tree change between renders. With _privateDisableTooltip={!text}, the Tooltip stays mounted but disabled when text is empty, so the tree is always stable.

Gotcha! We can go about adding it back then!

👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/15252

Integration test results from github/github-ui:

  CI Passed
  VRT Passed
  Projects Waiting